Add phx.gen.release task for release/docker based deployments by chrismccord · Pull Request #4609 · phoenixframework/phoenix

chrismccord · 2021-11-23T20:34:06Z

I'm not super firm on env names, so would love feedback. The migrate and server shims are necessary for deployments where it's difficult for infrastructure to know what the OTP app name is. If we are happy with this direction, we can add bat scripts for windows to match migrate/server.

chrismccord · 2021-11-23T20:41:30Z

The bin/server shim also aids folks who forget to set server: true in their config. This has been very frequent stumbling block over the years, so by generating runtime config that looks for the server env, we can also smooth over this particular stumbling block.

Co-authored-by: Michael Crumm <mike@crumm.net>

Gazler

Good idea, covers most of the common cases. Big +1 for the server: true change.

Gazler · 2021-11-24T08:09:43Z

          For example: ecto://USER:PASS@HOST/DATABASE
          \"""
+
+      ipv6? = System.get_env("IPV6") == "true"


Is IPV6 the correct environment variable here? Since this is specific to the db socket, wouldn't something like DATABASE_IPV6 be more appropriate?

Gazler · 2021-11-24T08:19:11Z

+    Your application is ready to be deployed in a release!
+
+        # To start your system
+        _build/dev/rel/live_beats/bin/live_beats start


s/live_beats/#{app}

Gazler · 2021-11-24T08:20:51Z

+    File.chmod!("rel/overlays/bin/migrate", 0o700)
+    File.chmod!("rel/overlays/bin/server", 0o700)
+
+    Mix.shell().info("""


Shouldn't this shell info explain something about building for Docker vs release information from mix?

Gazler · 2021-11-24T08:21:48Z

+    ])
+
+    Mix.Phoenix.copy_from(paths(), "priv/templates/phx.gen.docker/rel", binding, [
+      {:eex, "env.bat.eex", "rel/env.bat.eex"},


Is there a way we could leverage these from the existing mix release.init task to keep any changes in sync with the Elixir version?

I don't see why we need to generate them. Could we tell them to use mix release.init if necessary?

Alternatively, we call this mix phx.gen.release and support a --docker flag?

Gazler · 2021-11-24T08:29:02Z

+#   - https://pkgs.org/ - resource for finding needed packages
+#   - Ex: hexpm/elixir:1.12.3-erlang-24.1.4-debian-bullseye-20210902-slim
+#
+ARG BUILDER_IMAGE="hexpm/elixir:1.12.3-erlang-24.1.4-debian-bullseye-20210902-slim"


Should we leverage:

Application.spec(:elixir, :vsn) and :erlang.system_info(:version) (not sure how to get the minor version here)

So that the build image matches the elixir version in use?

This might have a downside as we can't ensure the image exists vs using a hard-coded string.

Good call. We can do -erlang-24.1-, which perhaps is the safest choice.

ZombieHarvester · 2021-11-24T08:48:57Z

+# note: if your project uses a tool like https://purgecss.com/,
+# which customizes asset compilation based on what it finds in
+# your Elixir templates, you will need to move the asset compilation
+# step down so that `lib` is available.
+COPY assets assets


Can asset compilation always be after lib? Won't it cover both cases with and without purgecss?

I don't know if there are any downsides of permanently moving it a step down, but this could make the generated dockerfile clearer.

The downside is slower builds when you change something in lib. This would trigger and asset recompilation.

There's a way to do both with a multistage Dockerfile though, it's a good idea.

josevalim · 2021-11-25T12:32:04Z

+  server? = System.get_env("SERVER") == "true"
+
  config :<%= @app_name %>, <%= @endpoint_module %>,
+    url: [host: host, port: 80],


Not sure if I am happy with the hardcoded port here. I am wondering if instead we could support url as a string which we will parse. So we could do: url: System.get_env("PHX_URL"), and we set PHX_URL=example.com:80. The code would parse the string and discard all nil fields?

josevalim · 2021-11-25T13:01:36Z

+  @doc false
+  def run(_args) do
+    if Mix.Project.umbrella?() do
+      Mix.raise("mix phx.gen.docker is not supported in umbrella applications")


Maybe mention they should generate it within their web app instead?

chrismccord · 2021-12-03T20:40:28Z

Updates:

The task has been renamed to phx.gen.release with an optional --docker flag for the Dockerfile.
Env vars included in config/runtime.exs for new apps are: ECTO_IPV6, PHX_HOST, and PHX_SERVER
Warnings are printed after the task is run if above env vars are not found in the file with instructions
Default :url port for production is set to 443, which is something we should have defaulted to a long time ago
The mix release.init files are no longer generated as they are not necessary and a user can opt-in to them in the future by running mix release.init

josevalim · 2021-12-06T09:42:51Z

+        Mix.shell().info(msg)
+    end
+  end
+end


This file is gorgeous.

josevalim · 2021-12-06T09:48:27Z

@@ -0,0 +1,3 @@
+#!/bin/sh
+cd -P -- "$(dirname -- "$0")"
+PHX_SERVER=true ./<%= otp_app %> start


Similar here:

Suggested change

PHX_SERVER=true ./<%= otp_app %> start

exec PHX_SERVER=true ./<%= otp_app %> start

I don't know if exec should come before or after the env var.

Co-authored-by: José Valim <jose.valim@dashbit.co>

josevalim · 2021-12-06T21:19:25Z

+        IO.warn("unable to read OTP minor version at #{path}. Falling back to #{major}.0.0")
+        "#{major}.0.0"


Suggested change

IO.warn("unable to read OTP minor version at #{path}. Falling back to #{major}.0.0")

"#{major}.0.0"

IO.warn("unable to read OTP minor version at #{path}. Falling back to #{major}.0")

"#{major}.0"

josevalim · 2021-12-06T21:22:06Z

+    with {:ok, content} <- File.read(path),
+         {:ok, %Version{} = vsn} <- Version.parse(String.trim_trailing(content)) do
+      "#{vsn.major}.#{vsn.minor}.#{vsn.patch}"


OTP versioning is... tricky. There is no 24.1.0, it is just 24.1. But i guess you could do this:

Suggested change

with {:ok, content} <- File.read(path),

{:ok, %Version{} = vsn} <- Version.parse(String.trim_trailing(content)) do

"#{vsn.major}.#{vsn.minor}.#{vsn.patch}"

with {:ok, content} <- File.read(path) do

String.trim(content)

Co-authored-by: José Valim <jose.valim@dashbit.co>

Add phx.gen.docker task for docker release based deployments

8b2be4c

chrismccord requested review from Gazler, jeregrine, josevalim and mcrumm November 23, 2021 20:34

Typo

3cbd9a3

mcrumm reviewed Nov 23, 2021

View reviewed changes

Comment thread priv/templates/phx.gen.docker/Dockerfile.eex Outdated

Update priv/templates/phx.gen.docker/Dockerfile.eex

187cc26

Co-authored-by: Michael Crumm <mike@crumm.net>

Gazler reviewed Nov 24, 2021

View reviewed changes

ZombieHarvester reviewed Nov 24, 2021

View reviewed changes

josevalim reviewed Nov 25, 2021

View reviewed changes

Comment thread installer/templates/phx_single/config/runtime.exs Outdated

josevalim reviewed Nov 25, 2021

View reviewed changes

Comment thread lib/mix/tasks/phx.gen.docker.ex Outdated

josevalim reviewed Nov 25, 2021

View reviewed changes

Comment thread installer/templates/phx_single/config/runtime.exs Outdated

josevalim reviewed Nov 25, 2021

View reviewed changes

chrismccord added 3 commits December 2, 2021 13:47

Use export prefixes and add post install warnings

e00d60f

Fix env reference

095f8b0

Rename to gen.release

21bb0db

chrismccord changed the title ~~Add phx.gen.docker task for docker release based deployments~~ Add phx.gen.release task for release/docker based deployments Dec 3, 2021

Fix module name

056f56c

jsierles mentioned this pull request Dec 3, 2021

WIP: Updates for upcoming v1.6.3 with a Docker generator task fly-apps/hello_elixir#6

Closed

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread installer/lib/phx_new/generator.ex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread installer/templates/phx_single/config/runtime.exs Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread priv/templates/phx.gen.release/Dockerfile.eex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread lib/mix/tasks/phx.gen.release.ex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread priv/templates/phx.gen.release/rel/migrate.sh.eex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

chrismccord and others added 5 commits December 6, 2021 13:13

Update installer/lib/phx_new/generator.ex

42db9e4

Co-authored-by: José Valim <jose.valim@dashbit.co>

Update installer/templates/phx_single/config/runtime.exs

0b75a0c

Co-authored-by: José Valim <jose.valim@dashbit.co>

Use new ipv6 reference

251905b

Only generate ecto files if ecto is present

3985c47

Use system elixir and erlang OTP versions in Dockerfile

e2f58cf

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread lib/mix/tasks/phx.gen.release.ex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread lib/mix/tasks/phx.gen.release.ex Outdated

josevalim reviewed Dec 6, 2021

View reviewed changes

Comment thread lib/mix/tasks/phx.gen.release.ex Outdated

chrismccord and others added 8 commits December 7, 2021 10:27

Fix otp version

96411dd

Update priv/templates/phx.gen.release/rel/migrate.sh.eex

b430ce4

Co-authored-by: José Valim <jose.valim@dashbit.co>

Update lib/mix/tasks/phx.gen.release.ex

dfa622c

Co-authored-by: José Valim <jose.valim@dashbit.co>

Update lib/mix/tasks/phx.gen.release.ex

973a306

Co-authored-by: José Valim <jose.valim@dashbit.co>

Tweaks

ad53ff0

Fix tests

42f9c89

Add bat overlays

b5ddcc1

Add missing scripts

6b26b74

chrismccord merged commit 380a281 into master Dec 7, 2021

chrismccord deleted the cm-gen-docker branch December 7, 2021 19:33

	PHX_SERVER=true ./<%= otp_app %> start
	exec PHX_SERVER=true ./<%= otp_app %> start

		IO.warn("unable to read OTP minor version at #{path}. Falling back to #{major}.0.0")
		"#{major}.0.0"

Uh oh!

Conversation

chrismccord commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrismccord commented Nov 23, 2021

Uh oh!

Uh oh!

Gazler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismccord commented Dec 3, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chrismccord commented Nov 23, 2021 •

edited

Loading